Skip to content

build: remove program headers from first loadable segment#586

Merged
badochov merged 1 commit intomasterfrom
badochov/remove_program_headers
Sep 17, 2024
Merged

build: remove program headers from first loadable segment#586
badochov merged 1 commit intomasterfrom
badochov/remove_program_headers

Conversation

@badochov
Copy link
Contributor

@badochov badochov commented Sep 9, 2024 •
edited
Loading

Program headers should not be loaded into kernel as there is no use for them. Previously starting kernel at start of memory region required explicit linker script.

JIRA: RTOS-908

Description

On all platform but ARM, program headers are placed in loadable segment before _init section. Resulting from this _init section start address isn't start address of first loadable segment. Due to that in PLO we may not be able to find proper segment for kernel if an address of the section that the kernel is to be loaded to = VADDR_KERNEL_INIT. Because of this additional linker script was required for sparc in kernel.

On ARM we headers weren't loaded most probably because of a bug(or very fortunated accident) in our binutils patch.
THe change introduced new file armelf_phoenix.sh but did't use it (ld/configure.tgt), instead it used armelf by default which defines EMBEDDED to true resulting in headers not being included.

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: (list targets here).

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

  • This PR needs additional PRs to work (list the PRs, preferably in merge-order).
  • I will merge this PR by myself when appropriate.

github-actions[bot]
github-actions bot reviewed Sep 9, 2024
View reviewed changes
@badochov badochov force-pushed the badochov/remove_program_headers branch from f8853f9 to 0925371 Compare September 9, 2024 19:12
@badochov badochov changed the title fix(build): remove program headers from first loadable section fix(build): remove program headers from first loadable segment Sep 9, 2024
@github-actions
Copy link

github-actions bot commented Sep 9, 2024 •
edited
Loading

Unit Test Results

7 700 tests  ±0   6 985 ✅ ±0   38m 50s ⏱️ -29s
  436 suites ±0     715 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 682c77d. ± Comparison against base commit b8de910.

♻️ This comment has been updated with latest results.

lukileczo
lukileczo reviewed Sep 10, 2024
View reviewed changes
@badochov badochov force-pushed the badochov/remove_program_headers branch from 0925371 to 388f411 Compare September 10, 2024 09:17
@badochov badochov requested a review from lukileczo September 10, 2024 09:24
@badochov badochov marked this pull request as ready for review September 10, 2024 09:24
lukileczo
lukileczo approved these changes Sep 10, 2024
View reviewed changes
Copy link
Member

@lukileczo lukileczo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nalajcie
Copy link
Member

nalajcie commented Sep 10, 2024

please don't use semantic commit messages, this is not in our coding standard (they don't help creating changelog in case of superproject)

build: remove program headers from first loadable segment
682c77d 
Program headers should not be loaded into kernel as there is no use for
them. Previously starting kernel at start of memory region required
explicit linker script.

JIRA: RTOS-910
@badochov badochov force-pushed the badochov/remove_program_headers branch from 388f411 to 682c77d Compare September 10, 2024 10:43
@badochov badochov changed the title fix(build): remove program headers from first loadable segment build: remove program headers from first loadable segment Sep 10, 2024
@badochov
Copy link
Contributor Author

badochov commented Sep 10, 2024

please don't use semantic commit messages, this is not in our coding standard (they don't help creating changelog in case of superproject)

FIxed

@nalajcie
Copy link
Member

nalajcie commented Sep 10, 2024

please don't use semantic commit messages, this is not in our coding standard (they don't help creating changelog in case of superproject)

FIxed

Thanks.
Just for reference: we might think about semantic commit messages if/when we will switch to monorepo, right now we do generate changelog from the PR descriptions (Types of changes section) - which is not ideal (as one PR might fix/add new feature simultaneously)) - for example: https://github.com/phoenix-rtos/phoenix-rtos-project/releases/tag/v3.2.0

This was referenced Sep 11, 2024
Merged
Closed
@badochov badochov merged commit 469212d into master Sep 17, 2024
@badochov badochov deleted the badochov/remove_program_headers branch September 17, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@lukileczo lukileczo lukileczo approved these changes

@agkaminski agkaminski Awaiting requested review from agkaminski

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@badochov @nalajcie @lukileczo